Build: Add stories for Main component
#33511
Conversation
|
View your CI Pipeline Execution ↗ for commit 839439a
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughAdds a MemoryRouter export and exposes it via globals; makes the manager Main component public; adds a new manager stories module that provides a ReactProvider with mock channel/addon/index wiring, WS disconnect handling, Storage API overrides, initialization logic, and ~12 interactive manager stories; updates Chromatic config and bumps a dependency. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Story as Story (play)
participant Provider as ReactProvider
participant Channel as Addon Channel
participant Addons as AddonStore
participant WS as WebSocket
participant UI as Manager UI
User->>Story: trigger play()
Story->>Provider: call navigation / API handlers
Provider->>Channel: emit/read addon events
Channel->>Addons: provide addon metadata
Provider->>WS: observe connection state
WS-->>Provider: disconnect / timeout event
Provider->>UI: enqueue connection notification
UI->>User: render updated manager state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{js,jsx,ts,tsx,json,md,html,css,scss}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,jsx,json,html,ts,tsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-10-02T09:22:13.215ZApplied to files:
📚 Learning: 2025-09-29T13:20:23.346ZApplied to files:
📚 Learning: 2025-11-05T09:38:47.712ZApplied to files:
📚 Learning: 2025-09-17T07:31:04.432ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.123ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✏️ Tip: You can disable this entire section by setting Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/core/src/manager/index.stories.tsx (2)
68-95: Unhandled promise rejection inhandleAPI.The
api.setIndex()call returns a Promise with a.then()handler, but no error handling is present. IfsetIndexfails, the error will be silently swallowed or cause an unhandled rejection.Proposed fix
handleAPI(api: API) { this.addons.loadAddons(api); // Initialize story index with mock data - api.setIndex(mockStoryIndex).then(() => { + api.setIndex(mockStoryIndex).then(() => { // Mark preview as initialized so the iframe doesn't show a spinner api.setPreviewInitialized(); // Set the current story to example-button--primary after the index is initialized // This navigates to the story URL, which will cause the iframe to load the correct story api.selectStory('example-button--primary', undefined, { viewMode: 'story' }); - }); + }).catch((err) => { + console.error('Failed to initialize story index:', err); + });
98-103: Consider creating a fresh provider per story for better isolation.The
ReactProviderinstance inargsis shared across all stories. SinceReactProviderhas mutable state (wsDisconnected) and the channel accumulates event handlers, stories likeConnectionLostandServerTimedOutmay affect subsequent stories if they run in sequence.Consider using a function that returns a new provider instance, or resetting state in
beforeEach:args: { provider: undefined, // Set dynamically }, beforeEach: () => { // ... existing code ... return { provider: new ReactProvider() }; },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/core/src/manager/globals/exports.tscode/core/src/manager/index.stories.tsxcode/core/src/manager/index.tsxcode/core/src/router/router.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/router/router.tsxcode/core/src/manager/index.tsxcode/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/router/router.tsxcode/core/src/manager/index.tsxcode/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/router/router.tsxcode/core/src/manager/index.tsxcode/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/router/router.tsxcode/core/src/manager/index.tsxcode/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/router/router.tsxcode/core/src/manager/index.tsxcode/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{renderers}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/client-logger` for client-side logging in browser code
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/manager/index.stories.tsx
🧬 Code graph analysis (1)
code/core/src/manager/index.stories.tsx (5)
code/core/src/manager/index.tsx (2)
Provider(101-101)Main(40-90)code/core/src/manager-api/root.tsx (2)
AddonStore(514-514)API(95-109)code/core/src/types/modules/addons.ts (1)
Addon_Types(26-29)code/core/src/theming/base.ts (1)
color(1-42)code/core/src/router/router.tsx (1)
MemoryRouter(145-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (5)
code/core/src/router/router.tsx (1)
145-145: LGTM!The
MemoryRouterexport follows the same passthrough pattern asLocationProviderandBaseLocationProviderabove, maintaining consistency in the router module's public API.code/core/src/manager/globals/exports.ts (1)
651-651: LGTM!The
MemoryRouterexport is correctly added to the router module's global exports, maintaining alphabetical order in the list.code/core/src/manager/index.tsx (1)
40-40: LGTM!Exporting
Mainenables its use in stories while preserving existing behavior inRoot. The change is minimal and appropriate for the PR's goal of enabling Storybook UI stories.code/core/src/manager/index.stories.tsx (2)
132-205: Well-structured stories with good coverage.The stories comprehensively cover key UI interactions and states including layout toggles, fullscreen mode, share menu, connection states, and settings pages. The play functions appropriately use
canvasfor component-scoped queries andscreenfor portaled content.
1-18: Missing React import for JSX usage.JSX is used at line 91 (
<FailedIcon color={color.negative} />) and in decorators (lines 120-125), but React is not imported. While some build configurations support automatic JSX runtime, explicit import ensures compatibility.Proposed fix
import { CHANNEL_CREATED, CHANNEL_WS_DISCONNECT } from 'storybook/internal/core-events'; import { MemoryRouter } from 'storybook/internal/router'; import type { Addon_Config, Addon_Types } from 'storybook/internal/types'; import type { API_PreparedStoryIndex } from 'storybook/internal/types'; + +import React from 'react';⛔ Skipped due to learnings
Learnt from: Sidnioulz Repo: storybookjs/storybook PR: 32458 File: code/core/src/components/components/Select/Select.tsx:200-204 Timestamp: 2025-11-05T09:38:47.712Z Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.Learnt from: Sidnioulz Repo: storybookjs/storybook PR: 32594 File: code/core/src/components/components/Popover/WithPopover.tsx:7-9 Timestamp: 2025-10-01T15:24:01.060Z Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.Learnt from: ndelangen Repo: storybookjs/storybook PR: 32507 File: code/core/src/manager/globals/globals-module-info.ts:25-33 Timestamp: 2025-09-24T09:39:39.233Z Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.Learnt from: Sidnioulz Repo: storybookjs/storybook PR: 32458 File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96 Timestamp: 2025-11-05T09:37:25.920Z Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.Learnt from: Sidnioulz Repo: storybookjs/storybook PR: 32458 File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227 Timestamp: 2025-11-05T09:36:55.944Z Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.Learnt from: CR Repo: storybookjs/storybook PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-22T22:03:40.123Z Learning: Applies to code/{renderers}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/client-logger` for client-side logging in browser code
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @code/core/src/manager/index.stories.tsx:
- Around line 155-160: The play function for the RightPanel story calls
within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement)
without checking for null; update the RightPanel -> meta.story play handler to
first store the result of canvasElement.querySelector('#storybook-panel-root')
in a variable, add a defensive null check (e.g., if the element is null return
or throw a clear error), and only then pass the element to within — this removes
the unsafe as HTMLElement cast and prevents a runtime error when querySelector
returns null.
🧹 Nitpick comments (1)
code/core/src/manager/index.stories.tsx (1)
72-79: Consider adding error handling for the promise chain.The
setIndexpromise chain lacks a.catch()handler. IfsetIndexor subsequent calls fail, errors will be silently swallowed. While this is test/story code, unhandled promise rejections could make debugging harder.♻️ Suggested improvement
api.setIndex(mockStoryIndex).then(() => { // Mark preview as initialized so the iframe doesn't show a spinner api.setPreviewInitialized(); // Set the current story to example-button--primary after the index is initialized // This navigates to the story URL, which will cause the iframe to load the correct story api.selectStory('example-button--primary', undefined, { viewMode: 'story' }); - }); + }).catch((error) => { + console.error('Failed to initialize story index:', error); + });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager/index.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/manager/index.stories.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/manager/index.stories.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/manager/index.stories.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/manager/index.stories.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/manager/index.stories.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/index.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/manager/index.stories.tsx (2)
98-103: Shared provider instance may cause state leakage between stories.The
ReactProvideris instantiated once inargsand shared across all stories. SinceReactProvidermaintains mutable state (wsDisconnected, channel listeners), running stories in sequence (e.g., in test runner) could cause cross-contamination. Consider creating a fresh provider per story or resetting state inbeforeEach.If isolation is confirmed to not be required due to how Storybook's test runner handles story args, this can be safely ignored.
132-205: Good coverage of UI states and interactions.The stories provide comprehensive coverage of the manager UI's interactive states. The use of
canvas.findBy*for elements within the story andscreen.findBy*for portaled elements (like menus) follows Storybook Testing Library best practices.
ghengeveld
force-pushed
the
storybook-root-ui-stories
branch
from
d59c7bb to
42f0107
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @code/core/src/manager/index.stories.tsx:
- Around line 156-159: The play function uses
within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement)
which will throw a confusing runtime error if querySelector returns null; change
it to first assign const panelRoot =
canvasElement.querySelector('#storybook-panel-root'); then guard it (if
(!panelRoot) throw new Error('storybook panel root (#storybook-panel-root) not
found in play()')); finally call within(panelRoot) and proceed with await
userEvent.click(await panel.findByLabelText('Move addon panel to right')); this
ensures a clear error or safe execution when the element is missing.
🧹 Nitpick comments (5)
code/core/src/manager/index.stories.tsx (5)
21-21: Shared module-level channel may cause state leakage between stories.The
channelis created once at module load and shared across all stories. Event listeners registered viachannel.on()inhandleAPIwill accumulate across story renders, potentially causing duplicate notifications or unexpected behavior when running multiple stories in sequence.Consider creating a fresh channel per story via
beforeEach, or clearing listeners inafterEach.
47-47: Unused field:wsDisconnectedis set but never read.The
wsDisconnectedproperty is assigned in theCHANNEL_WS_DISCONNECThandler (line 83) but is never consumed anywhere in the code. Consider removing it if it's not needed, or document its intended purpose for future use.
72-79: Consider adding error handling for the async operation.The promise returned by
api.setIndex()has no.catch()handler. If the index initialization fails, the error will be silently swallowed, making debugging difficult.♻️ Suggested improvement
// Initialize story index with mock data - api.setIndex(mockStoryIndex).then(() => { + api.setIndex(mockStoryIndex).then(() => { // Mark preview as initialized so the iframe doesn't show a spinner api.setPreviewInitialized(); // Set the current story to example-button--primary after the index is initialized // This navigates to the story URL, which will cause the iframe to load the correct story api.selectStory('example-button--primary', undefined, { viewMode: 'story' }); - }); + }).catch((error) => { + console.error('Failed to initialize story index:', error); + });
101-103: Provider instance is shared across all stories.The
ReactProvideris instantiated once at meta definition time and shared across all story variants. Combined with the shared module-level channel, this can cause state accumulation between stories (e.g., multipleCHANNEL_WS_DISCONNECTlisteners).For better story isolation, consider using a function that returns a fresh provider:
♻️ Suggested approach
args: { - provider: new ReactProvider(), + provider: undefined as unknown as ReactProvider, }, + beforeEach: () => { + // Create fresh provider per story + return { provider: new ReactProvider() }; + },Alternatively, use
renderorloadersto create a fresh provider per story execution.
120-125: Unnecessarykeyprops on decorator elements.The
keyprops onHelmetProviderandMemoryRouterare not needed since these elements are not rendered in a list or array context. React keys are only necessary for reconciliation in dynamic lists.♻️ Suggested cleanup
decorators: [ (Story) => ( - <HelmetProvider key="helmet.Provider"> - <MemoryRouter key="location.provider"> + <HelmetProvider> + <MemoryRouter> <Story /> </MemoryRouter> </HelmetProvider> ), ],
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager/index.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/manager/index.stories.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/manager/index.stories.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/manager/index.stories.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/manager/index.stories.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/manager/index.stories.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/index.stories.tsx
🧬 Code graph analysis (1)
code/core/src/manager/index.stories.tsx (5)
code/core/src/manager/index.tsx (2)
Provider(101-101)Main(40-90)code/core/src/manager-api/root.tsx (2)
AddonStore(514-514)API(95-109)code/core/src/types/modules/addons.ts (1)
Addon_Types(26-29)code/core/src/theming/base.ts (1)
color(1-42)code/core/src/router/router.tsx (1)
MemoryRouter(145-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (3)
code/core/src/manager/index.stories.tsx (3)
1-17: LGTM!The imports are well-organized and appropriate for the stories module, correctly pulling in internal Storybook APIs, testing utilities, and the necessary components.
27-40: LGTM!The mock story index is well-structured with the necessary fields for a prepared story index, providing a minimal but sufficient fixture for testing the manager UI.
132-205: LGTM!The story variants provide good coverage of the manager UI interactions including sidebar/toolbar/panel toggles, full screen mode, share menu, connection states, and settings pages. The play functions follow consistent patterns using Testing Library queries.
| play: async ({ canvasElement, userEvent }) => { | ||
| const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement); | ||
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | ||
| }, |
There was a problem hiding this comment.
Potential null reference in querySelector assertion.
If the #storybook-panel-root element doesn't exist when the play function runs, querySelector returns null, and within(null as HTMLElement) will cause a runtime error with a confusing message.
Consider using a safer approach:
🐛 Suggested fix
export const RightPanel = meta.story({
play: async ({ canvasElement, userEvent }) => {
- const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement);
+ const panelRoot = canvasElement.querySelector('#storybook-panel-root');
+ if (!panelRoot) {
+ throw new Error('Panel root element not found');
+ }
+ const panel = within(panelRoot as HTMLElement);
await userEvent.click(await panel.findByLabelText('Move addon panel to right'));
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| play: async ({ canvasElement, userEvent }) => { | |
| const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement); | |
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | |
| }, | |
| play: async ({ canvasElement, userEvent }) => { | |
| const panelRoot = canvasElement.querySelector('#storybook-panel-root'); | |
| if (!panelRoot) { | |
| throw new Error('Panel root element not found'); | |
| } | |
| const panel = within(panelRoot as HTMLElement); | |
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | |
| }, |
🤖 Prompt for AI Agents
In @code/core/src/manager/index.stories.tsx around lines 156 - 159, The play
function uses within(canvasElement.querySelector('#storybook-panel-root') as
HTMLElement) which will throw a confusing runtime error if querySelector returns
null; change it to first assign const panelRoot =
canvasElement.querySelector('#storybook-panel-root'); then guard it (if
(!panelRoot) throw new Error('storybook panel root (#storybook-panel-root) not
found in play()')); finally call within(panelRoot) and proceed with await
userEvent.click(await panel.findByLabelText('Move addon panel to right')); this
ensures a clear error or safe execution when the element is missing.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @code/core/src/manager/index.stories.tsx:
- Around line 156-161: The play function in RightPanel calls
canvasElement.querySelector('#storybook-panel-root') and force-casts to
HTMLElement which can be null; update the play implementation to first capture
the result into a variable, check for null (e.g., if (!panelRoot) return or
throw a clear error), only then cast to HTMLElement and call within(panelRoot);
ensure subsequent calls (panel.findByLabelText) are only invoked when panelRoot
exists so you avoid downstream null dereferences in meta.story's play.
- Around line 108-119: Store the original global.PREVIEW_URL before mutating it
in the beforeEach (e.g., const originalPreviewUrl = global.PREVIEW_URL) and then
restore it in afterEach by assigning global.PREVIEW_URL = originalPreviewUrl;
update the beforeEach/afterEach blocks around the existing Storage.prototype
mocks so the test suite doesn't leak the preview URL state.
🧹 Nitpick comments (2)
code/core/src/manager/index.stories.tsx (2)
48-48: Unused property:wsDisconnectedis set but never read.The property is assigned on line 84 but has no usages. Either remove it or clarify its intended purpose.
🔧 Suggested fix
class ReactProvider extends Provider { addons: AddonStore; channel: Channel; - wsDisconnected = false;
73-80: Consider adding error handling to the promise chain.If
setIndexrejects, the error will be silently swallowed and subsequent initialization steps won't execute.🔧 Suggested fix
// Initialize story index with mock data - api.setIndex(mockStoryIndex).then(() => { + api.setIndex(mockStoryIndex).then(() => { // Mark preview as initialized so the iframe doesn't show a spinner api.setPreviewInitialized(); // Set the current story to example-button--primary after the index is initialized // This navigates to the story URL, which will cause the iframe to load the correct story api.selectStory('example-button--primary', undefined, { viewMode: 'story' }); - }); + }).catch((error) => { + console.error('Failed to initialize story index:', error); + });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/core/src/manager/globals/exports.tscode/core/src/manager/index.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/index.stories.tsxcode/core/src/manager/globals/exports.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/index.stories.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/index.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (5)
code/core/src/manager/globals/exports.ts (1)
646-662: LGTM!The
MemoryRouterexport is correctly added in alphabetical order and aligns with the new story file's decorator requirements.code/core/src/manager/index.stories.tsx (4)
1-18: LGTM!Imports are well-organized and necessary for the story setup.
20-41: LGTM!The mock channel setup and story index provide sufficient test infrastructure for the manager UI stories.
120-128: LGTM!The decorator correctly wraps stories with the necessary routing and helmet context.
163-206: LGTM!The interactive stories effectively cover various UI states and use appropriate query strategies—
canvasfor component elements andscreenfor portal elements.
| beforeEach: () => { | ||
| global.PREVIEW_URL = 'about:blank'; | ||
|
|
||
| Storage.prototype.getItem = () => null; | ||
| Storage.prototype.setItem = () => {}; | ||
| Storage.prototype.clear = () => {}; | ||
| }, | ||
| afterEach: () => { | ||
| Storage.prototype.getItem = originalGetItem; | ||
| Storage.prototype.setItem = originalSetItem; | ||
| Storage.prototype.clear = originalClear; | ||
| }, |
There was a problem hiding this comment.
global.PREVIEW_URL is not restored in afterEach.
The beforeEach hook sets global.PREVIEW_URL = 'about:blank' but afterEach doesn't restore the original value, which could leak state to other tests.
🔧 Suggested fix
+const originalPreviewUrl = global.PREVIEW_URL;
+
const meta = preview.meta({
// ...
beforeEach: () => {
global.PREVIEW_URL = 'about:blank';
Storage.prototype.getItem = () => null;
Storage.prototype.setItem = () => {};
Storage.prototype.clear = () => {};
},
afterEach: () => {
+ global.PREVIEW_URL = originalPreviewUrl;
Storage.prototype.getItem = originalGetItem;
Storage.prototype.setItem = originalSetItem;
Storage.prototype.clear = originalClear;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach: () => { | |
| global.PREVIEW_URL = 'about:blank'; | |
| Storage.prototype.getItem = () => null; | |
| Storage.prototype.setItem = () => {}; | |
| Storage.prototype.clear = () => {}; | |
| }, | |
| afterEach: () => { | |
| Storage.prototype.getItem = originalGetItem; | |
| Storage.prototype.setItem = originalSetItem; | |
| Storage.prototype.clear = originalClear; | |
| }, | |
| const originalGetItem = Storage.prototype.getItem; | |
| const originalSetItem = Storage.prototype.setItem; | |
| const originalClear = Storage.prototype.clear; | |
| const originalPreviewUrl = global.PREVIEW_URL; | |
| const meta = preview.meta({ | |
| // ... other configuration | |
| beforeEach: () => { | |
| global.PREVIEW_URL = 'about:blank'; | |
| Storage.prototype.getItem = () => null; | |
| Storage.prototype.setItem = () => {}; | |
| Storage.prototype.clear = () => {}; | |
| }, | |
| afterEach: () => { | |
| global.PREVIEW_URL = originalPreviewUrl; | |
| Storage.prototype.getItem = originalGetItem; | |
| Storage.prototype.setItem = originalSetItem; | |
| Storage.prototype.clear = originalClear; | |
| }, |
🤖 Prompt for AI Agents
In @code/core/src/manager/index.stories.tsx around lines 108 - 119, Store the
original global.PREVIEW_URL before mutating it in the beforeEach (e.g., const
originalPreviewUrl = global.PREVIEW_URL) and then restore it in afterEach by
assigning global.PREVIEW_URL = originalPreviewUrl; update the
beforeEach/afterEach blocks around the existing Storage.prototype mocks so the
test suite doesn't leak the preview URL state.
| export const RightPanel = meta.story({ | ||
| play: async ({ canvasElement, userEvent }) => { | ||
| const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement); | ||
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add null check for querySelector result.
querySelector can return null if the element doesn't exist. The direct cast to HTMLElement masks this, leading to confusing errors downstream.
🔧 Suggested fix
export const RightPanel = meta.story({
play: async ({ canvasElement, userEvent }) => {
- const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement);
+ const panelRoot = canvasElement.querySelector('#storybook-panel-root');
+ if (!panelRoot) {
+ throw new Error('Panel root element not found');
+ }
+ const panel = within(panelRoot as HTMLElement);
await userEvent.click(await panel.findByLabelText('Move addon panel to right'));
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const RightPanel = meta.story({ | |
| play: async ({ canvasElement, userEvent }) => { | |
| const panel = within(canvasElement.querySelector('#storybook-panel-root') as HTMLElement); | |
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | |
| }, | |
| }); | |
| export const RightPanel = meta.story({ | |
| play: async ({ canvasElement, userEvent }) => { | |
| const panelRoot = canvasElement.querySelector('#storybook-panel-root'); | |
| if (!panelRoot) { | |
| throw new Error('Panel root element not found'); | |
| } | |
| const panel = within(panelRoot as HTMLElement); | |
| await userEvent.click(await panel.findByLabelText('Move addon panel to right')); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In @code/core/src/manager/index.stories.tsx around lines 156 - 161, The play
function in RightPanel calls
canvasElement.querySelector('#storybook-panel-root') and force-casts to
HTMLElement which can be null; update the play implementation to first capture
the result into a variable, check for null (e.g., if (!panelRoot) return or
throw a clear error), only then cast to HTMLElement and call within(panelRoot);
ensure subsequent calls (panel.findByLabelText) are only invoked when panelRoot
exists so you avoid downstream null dereferences in meta.story's play.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @code/package.json:
- Line 77: The package.json entry for "@chromatic-com/storybook" upgrades to v5
which has breaking changes; update package.json to replace the old CLI package
name "storybook-chromatic" with "chromatic" (remove any residual
"storybook-chromatic"), remove deprecated packages ("chromatic-cypress",
"chromatic-playwright", "@chromaui/archive-storybook",
"@chromaui/test-archiver") from dependencies/devDependencies, search for and
remove usage of the deprecated --preserve-missing flag in scripts or CI and
migrate those invocations to TurboSnap or use
"onlyChanged"/"onlyStoryFiles"/"onlyStoryNames" options, update
chromatic.config.json to match v5 configuration schema, and locally run yarn
build-storybook and a full Chromatic story extraction to verify stories are
extracted correctly before merging.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
code/chromatic.config.jsoncode/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/chromatic.config.jsoncode/package.json
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/chromatic.config.jsoncode/package.json
🧠 Learnings (6)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/package.json
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/package.json
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Use Node.js 22.21.1 (see `.nvmrc`) and Yarn 4.9.1 for development
Applied to files:
code/package.json
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/package.json
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (1)
code/chromatic.config.json (1)
3-7: LGTM!The
buildScriptNamecorrectly references the existingstorybook:ui:buildscript inpackage.json, andstorybookBaseDiris appropriately set to"code"since this config file resides in that directory. These additions properly configure Chromatic for visual testing of the new manager UI stories.
| ], | ||
| "dependencies": { | ||
| "@chromatic-com/storybook": "^4.1.3", | ||
| "@chromatic-com/storybook": "^5.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@chromatic-com/storybook v5.0.0 breaking changes migration
💡 Result:
Short summary — what changed and how to migrate from @chromatic-com/storybook → v5.x:
-
There’s a breaking-release series around v5.0.0 (published as pre/next tags). Check the release notes/changelog for exact breaking items before upgrading. [1][2]
-
Common breaking/migration actions Chromatic documents that are likely relevant when moving to v5:
- Replace legacy CLI/package names (storybook-chromatic → chromatic) and use the new chromatic CLI where applicable. [3]
- Remove deprecated Storybook-related Chromatic packages (e.g., chromatic-cypress, chromatic-playwright, @chromaui/archive-storybook, @chromaui/test-archiver) if present; follow the E2E upgrade guide. [4]
- Stop using the deprecated --preserve-missing flag (it’s being removed in the v5 capture infra); migrate to TurboSnap or use onlyStoryFiles/onlyStoryNames/onlyChanged options. [5]
- Verify Storybook versions / peer-deps and update Storybook itself if required by the new adapter. Run a local build (npm run build-storybook) and ensure stories extract correctly before CI. [4]
-
Practical upgrade steps
- Read the v5.x release notes/changelog for @chromatic-com/storybook and chromatic to catch any package-specific breakages. [1][2]
- Remove deprecated packages and flags listed above. [4][5]
- npm/yarn remove storybook-chromatic (if present) and npm/yarn add chromatic@latest (or the exact 5.x tag you need). [3]
- Run build-storybook locally; fix any story extraction or add-on incompatibilities; update Storybook peer deps if necessary. [4]
- Enable TurboSnap or configure onlyStoryFiles/onlyStoryNames/onlyChanged to replace preserve-missing behaviour. [5]
- Re-run Chromatic in CI; consult Chromatic docs / changelog for any CI-specific inputs that changed. [4][5]
If you want, I can:
- fetch the exact v5.0.0 release notes/changelog entries and list the line-by-line breaking changes, or
- inspect your repository’s package.json and Chromatic config and give step-by-step commands to migrate. Which would you prefer?
Sources:
[1] Release listing for @chromatic-com/storybook (v5.0.0-next.0).
[2] Chromatic blog / Storybook 5.0 overview and Chromatic changelog.
[3] chromatic (CLI) npm package notes about migrating from storybook-chromatic.
[4] Chromatic docs — Quickstart / Storybook integration and E2E Visual Tests upgrade guide.
[5] Chromatic docs — Sunsetting --preserve-missing and migration to TurboSnap.
🌐 Web query:
@chromatic-com/storybook v5 release notes changelog
💡 Result:
Summary — @chromatic-com/storybook v5 (pre-release)
- A v5 pre-release exists as 5.0.0-next.0 (marked as pushed to the
nextchannel). Noted items: breaking-change label, merges from main/3.2.7, and an update to the GraphQL schema to handle ComparisonResult.SKIPPED. [1][2]
Sources:
- NewReleases entry for @chromatic-com/storybook v5.0.0-next.0. [1]
- Package changelog / Yarn/npm package page showing recent changelog entries for @chromatic-com/storybook. [2]
- Storybook integrations page for the @chromatic-com/storybook addon (context about the package). [3]
Would you like links to the full release entry or the GitHub/ npm package pages?
🏁 Script executed:
#!/bin/bash
# Verify package exists on npm and check available versions
npm view @chromatic-com/storybook@5.0.0 version 2>/dev/null || echo "Package v5.0.0 not found"
npm view @chromatic-com/storybook dist-tags 2>/dev/nullRepository: storybookjs/storybook
Length of output: 232
Verify compatibility with @chromatic-com/storybook v5.0.0 breaking changes.
This major version upgrade (v4 → v5) includes breaking changes. Before merging, confirm:
- CLI package renamed:
storybook-chromatic→chromatic(remove old package if present) - Deprecated packages removed:
chromatic-cypress,chromatic-playwright,@chromaui/archive-storybook,@chromaui/test-archiver --preserve-missingflag no longer supported; migrate to TurboSnap oronlyChanged/onlyStoryFiles/onlyStoryNamesoptions- Verify
chromatic.config.jsonchanges align with v5 configuration expectations - Run
yarn build-storybooklocally and verify story extraction works before CI
🤖 Prompt for AI Agents
In @code/package.json at line 77, The package.json entry for
"@chromatic-com/storybook" upgrades to v5 which has breaking changes; update
package.json to replace the old CLI package name "storybook-chromatic" with
"chromatic" (remove any residual "storybook-chromatic"), remove deprecated
packages ("chromatic-cypress", "chromatic-playwright",
"@chromaui/archive-storybook", "@chromaui/test-archiver") from
dependencies/devDependencies, search for and remove usage of the deprecated
--preserve-missing flag in scripts or CI and migrate those invocations to
TurboSnap or use "onlyChanged"/"onlyStoryFiles"/"onlyStoryNames" options, update
chromatic.config.json to match v5 configuration schema, and locally run yarn
build-storybook and a full Chromatic story extraction to verify stories are
extracted correctly before merging.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 49 | 0 |
| Self size | 20.30 MB | 20.32 MB | 🚨 +18 KB 🚨 |
| Dependency size | 16.52 MB | 16.52 MB | 🎉 -4 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 159 | 0 |
| Self size | 30 KB | 30 KB | 🚨 +8 B 🚨 |
| Dependency size | 23.00 MB | 23.12 MB | 🚨 +115 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 🚨 +139 B 🚨 |
| Dependency size | 67.38 MB | 67.47 MB | 🚨 +87 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -4 B 🎉 |
| Dependency size | 65.95 MB | 66.04 MB | 🚨 +87 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 1000 KB | 999 KB | 🎉 -48 B 🎉 |
| Dependency size | 36.82 MB | 36.84 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | node | node |
Contributes to #33245
What I did
I've added stories for the
MainUI component, which is at the root of the Manager UI. This is a first step towards running Storybook's UI inside of Storybook itself.Right now it avoids rendering the preview iframe, because I have not yet been able to have it render a specific story (setting
global.PREVIEW_URLtoiframe.htmlmakes it render the ESBuild Metafiles story, because that happens to be the first story in the outer Storybook, whilst not setting it at all causes an infinitely nested Storybook, crashing the browser).Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
New Stories / Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.